Add unit test for history config #6334
Conversation
Shaddoll
requested review from
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
... and 28 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -308,7 +279,6 @@ type Config struct { | |||
| // EnableContextHeaderInVisibility whether to enable indexing context header in visibility | |||
| EnableContextHeaderInVisibility dynamicconfig.BoolPropertyFnWithDomainFilter | |||
|
|
|||
| EnableCrossClusterEngine dynamicconfig.BoolPropertyFn | |||
| EnableCrossClusterOperationsForDomain dynamicconfig.BoolPropertyFnWithDomainFilter | |||
There was a problem hiding this comment.
kinda surprising this doesn't go away, if the whole cross-cluster thing is gone
| "GlobalRatelimiterGCAfter": {dynamicconfig.HistoryGlobalRatelimiterGCAfter, time.Second}, | ||
| "HostName": {nil, hostname}, | ||
| } | ||
| client := dynamicconfig.NewInMemoryClient() | ||
| for fieldName, expected := range fields { | ||
| if expected.key != nil { | ||
| err := client.UpdateValue(expected.key, expected.value) | ||
| if err != nil { | ||
| t.Errorf("Failed to update config for %s: %s", fieldName, err) | ||
| } | ||
| } | ||
| } | ||
| dc := dynamicconfig.NewCollection(client, testlogger.New(t)) |
There was a problem hiding this comment.
I kinda wonder if this would be better to do as a "pick a hashed value for whatever key was requested"... the keys and values sorta don't matter, just that they're predictable and come from the dynamic config.
There was a problem hiding this comment.
the values have types. And the type matters.
What changed?
Why?
Improve test coverage
How did you test it?
unit tests
Potential risks
Release notes
Documentation Changes